fix: handle case where account entities is empty in after cache access #7329
Conversation
LuccaRebelloToledo
requested review from
sameerag,
tnorling,
hectormmg,
peterzenz,
bgavrilMS,
Robbie-Microsoft,
trwalke,
pmaytak,
gladjohn,
neha-bhargava,
rayluo and
Avery-Dunn
as code owners
|
@microsoft-github-policy-service agree |
| await this.client.set( | ||
| partitionKey, | ||
| cacheContext.tokenCache.serialize() | ||
| ); |
There was a problem hiding this comment.
Instead of refactoring everything, can you keep this as simple as possible? Lets move this out of the if statement (after it), and add an else for partitionKey = this.partitionManager.getKey()
There was a problem hiding this comment.
Do you mean something like that?
let partitionKey;
if (accountEntities.length > 0) {
const accountEntity = accountEntities[0] as AccountEntity;
partitionKey = await this.partitionManager.extractKey(
accountEntity
);
} else {
partitionKey = this.partitionManager.getKey()
}
await this.client.set(
partitionKey,
cacheContext.tokenCache.serialize()
);
|
@LuccaRebelloToledo This fix will require a new unit test in DistributedCachePlugin.spec.ts. |
@Robbie-Microsoft At this point I'm in doubt about how to run the unit test because I found that in TokenCache.spec.ts the data is not fully mocked and actually uses the TokenCache functions. Should I change to use it like in TokenCache.spec.ts or should I continue mocking the functions and results? |
…ing cache state consistency post-serialization
There was a problem hiding this comment.
Result of all tests:
Result of tests in DistributedCachePlugin.spec.ts
|
Thanks so much for highlighting this issue and proposing a fix @LuccaRebelloToledo |
Based on the Issue 7324, after implementing the DistributedCachePlugin class, when using the removeAccount method, the credentials were not cleared from my client cache. So, I started investigating and uploaded new personal versions of the library with logs during the entire account removal process and noticed that at the final moment of passing where the cache object was cleared, when trying to recover the KVStore cache, it was empty, that is, it was impossible to identify the partition key, preventing the clearance of the credentials in the client cache. However, I performed a validation where, if the accountEntities were empty, it would search for the partition key from my partition manager, correcting the problem of not clearing the credentials.
Previously, when using the removeAccount method, the accountEntities array was empty during the afterCacheAccess phase, preventing extraction of the partitionKey.
Now, if no accountEntities are available, the partitionKey is fetched from the partitionManager to ensure that the cache can still be updated.
Before the fix:
After the fix:
Test results:
